-
Notifications
You must be signed in to change notification settings - Fork 0
[feat/input] - input 컴포넌트 구현 #6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Summary of Changes
Hello @ramong26, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
이 Pull Request는 새로운 Input 컴포넌트를 시스템에 도입하는 것을 목표로 합니다. 이를 위해 컴포넌트의 기능적 구현, 시각적 스타일링, 개발자가 쉽게 참조할 수 있는 Storybook 예제, 그리고 컴포넌트의 안정성을 보장하는 단위 테스트가 함께 추가되었습니다. 또한, 기존 Button 및 Checkbox 컴포넌트의 스타일을 개선하여 전반적인 UI 일관성을 높였으며, 개발 환경 설정을 위한 의존성 및 워크스페이스 파일도 업데이트되었습니다.
Highlights
- 새로운 Input 컴포넌트 구현: React 기반의 Input 컴포넌트와 관련 스타일(Input.scss), Storybook 스토리(Input.stories.tsx), 그리고 단위 테스트(Input.test.tsx)가 추가되었습니다.
- 기존 컴포넌트 스타일 개선: Button 및 Checkbox 컴포넌트의 active 및 checked 상태 스타일이 일관성을 위해 조정되었습니다.
- 개발 의존성 업데이트: @types/testing-library__user-event 타입 정의가 package.json 및 pnpm-lock.yaml에 추가되었습니다.
- PNPM 워크스페이스 설정 추가: pnpm-workspace.yaml 파일이 새로 생성되어 ignoredBuiltDependencies 설정이 포함되었습니다.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in pull request comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR implements a new Input component for the design system with label functionality and 95 retro styling. The component follows the established design patterns from other components like Button and Checkbox.
- Adds a complete Input component with TypeScript support, tests, and Storybook documentation
- Updates existing component styling to use consistent color tokens instead of hardcoded values
- Adds development dependency for improved type safety in testing
Reviewed Changes
Copilot reviewed 9 out of 11 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/componets/Input/Input.tsx | Main Input component implementation with label and TypeScript interface |
| src/componets/Input/Input.test.tsx | Comprehensive test suite covering rendering, props, and events |
| src/componets/Input/Input.stories.tsx | Storybook configuration for component documentation |
| src/componets/Input/Input.scss | 95-style CSS with box-shadow effects and responsive design |
| src/componets/Checkbox/Checkbox.stories.tsx | Updated story names for better clarity |
| src/componets/Checkbox/Checkbox.scss | Replaced hardcoded color with design token |
| src/componets/Button/Button.scss | Replaced hardcoded colors with design tokens |
| pnpm-workspace.yaml | Added ignored build dependencies configuration |
| package.json | Added TypeScript definitions for testing library |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
src/componets/Input/Input.tsx
Outdated
| const Input: React.FC<InputProps> = ({ label, id, ...rest }) => { | ||
| return ( | ||
| <div className="input_wrapper"> | ||
| <input type="text" className="input_field" id={id} {...rest} /> | ||
| <label htmlFor={id} className="input_placeholder">{label}</label> |
Copilot
AI
Sep 16, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The component requires an id prop for proper label association but doesn't enforce this requirement. Consider making id required in the InputProps interface or generating a unique ID internally if none is provided.
| const meta: Meta<typeof Input> = { | ||
| title: 'Components/Input', | ||
| component: Input, | ||
| argTypes: { onClick: { action: 'changed' } } |
Copilot
AI
Sep 16, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The argTypes configuration references 'onClick' but Input components typically use 'onChange' for input events. This should be onChange: { action: 'changed' } to properly capture input change events in Storybook.
| argTypes: { onClick: { action: 'changed' } } | |
| argTypes: { onChange: { action: 'changed' } } |
src/componets/Input/Input.scss
Outdated
| background: #bbbbbb; | ||
| color: s.color(gray-900); | ||
| box-shadow: | ||
| inset 1px 1px 0 0 s.color(white), | ||
| 1px 1px 0 0 #868686, |
Copilot
AI
Sep 16, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hardcoded color values should be replaced with design system tokens for consistency. Use s.color(gray-300) instead of #bbbbbb and s.color(gray-600) instead of #868686 to match the pattern used in other parts of the codebase.
| background: #bbbbbb; | |
| color: s.color(gray-900); | |
| box-shadow: | |
| inset 1px 1px 0 0 s.color(white), | |
| 1px 1px 0 0 #868686, | |
| background: s.color(gray-300); | |
| color: s.color(gray-900); | |
| box-shadow: | |
| inset 1px 1px 0 0 s.color(white), | |
| 1px 1px 0 0 s.color(gray-600), |
src/componets/Input/Input.scss
Outdated
| background: #bbbbbb; | ||
| color: s.color(gray-900); | ||
| box-shadow: | ||
| inset 1px 1px 0 0 s.color(white), | ||
| 1px 1px 0 0 #868686, |
Copilot
AI
Sep 16, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hardcoded color values should be replaced with design system tokens for consistency. Use s.color(gray-300) instead of #bbbbbb and s.color(gray-600) instead of #868686 to match the pattern used in other parts of the codebase.
| background: #bbbbbb; | |
| color: s.color(gray-900); | |
| box-shadow: | |
| inset 1px 1px 0 0 s.color(white), | |
| 1px 1px 0 0 #868686, | |
| background: s.color(gray-300); | |
| color: s.color(gray-900); | |
| box-shadow: | |
| inset 1px 1px 0 0 s.color(white), | |
| 1px 1px 0 0 s.color(gray-600), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
이 PR은 새로운 Input 컴포넌트를 구현하고 기존 Button 및 Checkbox 컴포넌트의 스타일을 개선하는 내용을 담고 있습니다. 전반적으로 좋은 시도이지만, 몇 가지 개선점을 제안합니다. 우선, 불필요하고 deprecated된 npm 패키지가 추가되어 이를 제거해야 합니다. 새로 추가된 Input 컴포넌트는 type prop을 받도록 수정하고, useId 훅을 사용하여 접근성을 향상시키는 것이 좋습니다. 또한, SCSS 파일에 하드코딩된 색상 값을 디자인 시스템 변수로 대체하고, 중복되는 스타일을 정리하여 코드의 일관성과 유지보수성을 높일 필요가 있습니다. 마지막으로 Storybook 스토리의 이름과 argTypes를 더 명확하게 수정하는 것을 권장합니다.
package.json
Outdated
| "@testing-library/user-event": "^14.6.1", | ||
| "@types/react": "^19.1.10", | ||
| "@types/react-dom": "^19.1.7", | ||
| "@types/testing-library__user-event": "^4.2.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
src/componets/Input/Input.tsx
Outdated
| import React from 'react'; | ||
| import './Input.scss'; | ||
|
|
||
| export interface InputProps | ||
| extends React.InputHTMLAttributes<HTMLInputElement> { | ||
| label: string; | ||
| } | ||
|
|
||
| const Input: React.FC<InputProps> = ({ label, id, ...rest }) => { | ||
| return ( | ||
| <div className="input_wrapper"> | ||
| <input type="text" className="input_field" id={id} {...rest} /> | ||
| <label htmlFor={id} className="input_placeholder">{label}</label> | ||
| </div> | ||
| ); | ||
| }; | ||
|
|
||
| export default Input; No newline at end of file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Input 컴포넌트의 재사용성과 접근성을 높이기 위해 몇 가지 개선을 제안합니다.
typeprop 추가:input의type이"text"로 하드코딩되어 있어 재사용성이 떨어집니다.type을 prop으로 받아password,email등 다양한 타입의input으로 사용할 수 있도록 변경합니다. 기본값은 'text'로 설정합니다.useId로 접근성 보장:idprop이 제공되지 않을 경우,label과input의 연결이 끊어져 접근성이 저하됩니다. React 18의useId훅을 사용해id가 없으면 고유 ID를 자동으로 생성하여htmlFor와id를 항상 연결하도록 합니다.
import React, { useId } from 'react';
import './Input.scss';
export interface InputProps
extends React.InputHTMLAttributes<HTMLInputElement> {
label: string;
}
const Input: React.FC<InputProps> = ({ label, id: propId, type = 'text', ...rest }) => {
const reactId = useId();
const id = propId || reactId;
return (
<div className="input_wrapper">
<input type={type} className="input_field" id={id} {...rest} />
<label htmlFor={id} className="input_placeholder">{label}</label>
</div>
);
};
export default Input;
| &:active { | ||
| background: #bbbbbb; | ||
| color: s.color(gray-900); | ||
| box-shadow: | ||
| inset 1px 1px 0 0 s.color(white), | ||
| 1px 1px 0 0 #868686, | ||
| 0px 0px 0 0 s.color(gray-900); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
:active 상태에서 색상 값이 하드코딩되어 있습니다. 디자인 시스템의 일관성을 유지하고 유지보수를 용이하게 하기 위해 s.color() 함수를 사용하여 색상 변수를 참조하는 것이 좋습니다. Button.scss 파일에서 유사한 변경이 이루어진 것을 참고할 수 있습니다.
| &:active { | |
| background: #bbbbbb; | |
| color: s.color(gray-900); | |
| box-shadow: | |
| inset 1px 1px 0 0 s.color(white), | |
| 1px 1px 0 0 #868686, | |
| 0px 0px 0 0 s.color(gray-900); | |
| } | |
| &:active { | |
| background: s.color(gray-300); | |
| color: s.color(gray-900); | |
| box-shadow: | |
| inset 1px 1px 0 0 s.color(white), | |
| 1px 1px 0 0 s.color(gray-600), | |
| 0px 0px 0 0 s.color(gray-900); | |
| } |
| .input_field:focus + .input_placeholder, | ||
| .input_field:not(:placeholder-shown) + .input_placeholder { | ||
| @include s.text-style-extended(xs, 400, gray-900); | ||
| background: s.color(white); | ||
| padding: 0 4px; | ||
| } No newline at end of file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| const meta: Meta<typeof Input> = { | ||
| title: 'Components/Input', | ||
| component: Input, | ||
| argTypes: { onClick: { action: 'changed' } } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| export default meta; | ||
| type Story = StoryObj<typeof Input>; | ||
|
|
||
| export const Primary: Story = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📝 PR 제목
인풋 컴포넌트 추가
🛠 변경 내용
🎯 목적 / 이유
✅ 확인 사항